-
-
Notifications
You must be signed in to change notification settings - Fork 699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build the registration from using the userschema provided by backend #6435
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for plone-components canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammar fixes.
This PR still requires technical review.
packages/volto/src/components/theme/Register/Register.stories.jsx
Outdated
Show resolved
Hide resolved
packages/volto/src/components/theme/Register/Register.stories.jsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Steve Piercy <[email protected]>
Co-authored-by: Steve Piercy <[email protected]>
Co-authored-by: Steve Piercy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more instance, and I think it's GTG.
packages/volto/src/components/theme/Register/__snapshots__/Register.test.jsx.snap
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I am willing to make it enter in 18, but I need someone to test it and give feedback.
Some Cypress tests would be great too.
Any takers?
/cc @plone/volto-team
On the other hand, we can make it for 18.1 since it's a feature. |
We can check this indeed. |
I have added a cypress test that shows that the fields in the I have been checking also the implementation of the registration, and I see that we are using 2 settings But using them would mean to change the REST API to expose those settings (right now the Which means it is a larger project. |
@erral not sure why the test are breaking now, although it seems ok and legit. |
Mmmm, strange, it looks like the snapshot structure changed. If I update the snapshot I get a completely different structure. I will look at it more in depth. |
If your pull request closes an open issue, include the exact text below, immediately followed by the issue number. When your pull request gets merged, then that issue will close automatically.
Closes #6434